Add node metrics screen #1375
Add node metrics screen #1375
Conversation
str4d
added
S-waiting-on-review
str4d
force-pushed
the
1331-node-metrics
branch
from
7617dc3
to
90b9f99
Compare
|
I have rebased this PR on beta 2. |
|
☔ The latest upstream changes (presumably #1578) made this pull request unmergeable. Please resolve the merge conflicts. |
daira
force-pushed
the
1331-node-metrics
branch
from
90b9f99
to
5c60ee7
Compare
|
I rebased this on current master (post-#1578). |
| @@ -191,15 +192,20 @@ Value generate(const Array& params, bool fHelp) | |||
| std::function<bool(std::vector<unsigned char>)> validBlock = | |||
| [&pblock](std::vector<unsigned char> soln) { | |||
| pblock->nSolution = soln; | |||
| solutionTargetChecks.increment(); | |||
There was a problem hiding this comment.
This RPC method is only used for regtest, so it doesn't need any of the increment() calls. Does not block (these calls are harmless, just unnecessary).
|
utACK. |
|
utACK, looks like @daira added the extra count calls for tromp's solver. Note that the print statements in tromp's solver will interfere with the metrics screen output. I'll test it now and see if I'd need to fix it (which i can do now). |
|
It's fine to just comment out those print statements, I think. |
|
Yeah, that's what I'll do for now. |
|
Okay, it interferes with one of the auto-update lines, but doesn't obscure any information and doesn't break the UI. Still, I'll fix it now :) |
The screen is implemented using ANSI Escape sequences. Closes zcash#1331
|
Found a bug: the solver increment call for tromp's solver was added inside the solution for loop, so it is counting every found solution instead of every run. Rebasing on master to fix. |
|
Oops, should have checked that more carefully. |
|
re-utACK. (I checked the fix for the bug @str4d mentioned, and the last commit commenting out |
|
Assigning to @nathan-at-least for a decision, because @zookozcash really really really wants this merged, particularly as we also merged #1578 to include a faster miner (although that is not enabled by default, but everyone is going to use it anyway). |
|
@nathan-at-least said:
My decision is yes, provided it merges and tests cleanly now. @zkbot r+ |
|
📌 Commit dccc140 has been approved by |
|
💔 Test failed - zcash |
I just pushed the one-line fix to get the RPC tests running. Should I re-run? |
|
Gonna re-run @zkbot r+ |
|
📌 Commit d6feeef has been approved by |
|
⌛ Testing commit d6feeef with merge a884e5f... |
|
@zkbot cancel |
Author: Jack Grigg <jack@z.cash> Signed-off-by: Daira Hopwood <daira@jacaranda.org>
daira
force-pushed
the
1331-node-metrics
branch
from
d6feeef
to
02a4ace
Compare
|
Please don't r+ until I've checked something locally. (I want to check how the metrics screen interacts with |
|
OK, |
|
Smoke-tested ACK. @zkbot r+ |
|
📌 Commit 02a4ace has been approved by |
|
☀️ Test successful - zcash |
Continuation of #1336
Closes #1331